-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
backport: merge bitcoin#21148, #21327, #23970, #24021, #24543, #26844, #25325, #28165, partial bitcoin#20524, #26036, #27981 (networking backports: part 7) #6067
Conversation
This pull request has conflicts, please rebase. |
This pull request has conflicts, please rebase. |
, bitcoin#24177, bitcoin#24299, bitcoin#24917, bitcoin#22564, bitcoin#25349, bitcoin#25571, bitcoin#17487, bitcoin#26999, bitcoin#27011 (blockstorage backports: part 2) b658637 merge bitcoin#27011: Add simulation-based `CCoinsViewCache` fuzzer (Kittywhiskers Van Gogh) 1d0e410 merge bitcoin#26999: A few follow-ups to bitcoin#17487 (Kittywhiskers Van Gogh) 7d837ea merge bitcoin#17487: allow write to disk without cache drop (Kittywhiskers Van Gogh) 2c758f4 merge bitcoin#25571: Make mapBlocksUnknownParent local, and rename it (Kittywhiskers Van Gogh) 70a91e1 merge bitcoin#25349: CBlockIndex/CDiskBlockIndex improvements for safety, consistent behavior (Kittywhiskers Van Gogh) eca0a64 merge bitcoin#22564: Move mutable globals cleared in ::UnloadBlockIndex to BlockManager (Kittywhiskers Van Gogh) 916b3f0 merge bitcoin#24917: Make BlockManager::LoadBlockIndex private (Kittywhiskers Van Gogh) e10ca27 merge bitcoin#24299: UnloadBlockIndex and ChainstateManager::Reset thread safety cleanups (Kittywhiskers Van Gogh) 18aa55b merge bitcoin#24177: add missing thread safety lock assertions (Kittywhiskers Van Gogh) 678e67c merge bitcoin#24235: use stronger EXCLUSIVE_LOCKS_REQUIRED() (Kittywhiskers Van Gogh) edc665c merge bitcoin#24103: Replace RecursiveMutex m_cs_chainstate with Mutex, and rename it (Kittywhiskers Van Gogh) d19ffd6 merge bitcoin#22278: Add LIFETIMEBOUND to CScript where needed (Kittywhiskers Van Gogh) Pull request description: ## Additional Information * Dependent on #6097 * Dependency for #6067 * Test failures in `linux64_multiprocess-build` ([build](https://gitlab.com/dashpay/dash/-/jobs/7550924662)) and `linux64_tsan-test` ([build](https://gitlab.com/dashpay/dash/-/jobs/7550924666)) do not stem from this PR but are pre-existing failures in `develop` ([build](https://gitlab.com/dashpay/dash/-/jobs/7550859495), [build](https://gitlab.com/dashpay/dash/-/jobs/7550859499)). A fix for the build failures has been opened as a separate PR. ## Breaking Changes None observed. ## Checklist: - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)** - [x] I have added or updated relevant unit/integration/functional/e2e tests - [x] I have made corresponding changes to the documentation **(note: N/A)** - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: LGTM, utACK b658637 PastaPastaPasta: utACK b658637 Tree-SHA512: 1a9c3a41617af274db169db47a9c9fce7ba7ce0b2d68aa75617640a55da11a0fa095cb25ce6a2d38f06d3f6a6cc4c08cb4cf82dca4bdc74192e8882fd5f7052f
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be working with no issues.
light ACK 71a5e90
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally LGTM: 71a5e90; needs clang-format fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK d0faa3a
`p2p_ibd_txrelay.py` was introduced in bitcoin#19423 but not backported as Dash doesn't have feefilter capabilities but this backport has the test check for additional cases which are within Dash's capabilities, so the test has been committed in with the feefilter portions minimally stripped out
Preparation for backporting bitcoin#24543, which makes `State()` internal to `PeerManagerImpl`.
This backport excludes annotations for members introduced in bitcoin#25717 as it hasn't been backported yet.
The change was introduced as an optimization in 027a852 (dash#3398) but prevents the backport of bitcoin#26844 due to the inability to engage in binary expressions with iterators of `std::list`.
To allow for the removal of a node from `vReceivableNodes`, the collection of node pointers have been made into an `std::set`. Marking as partial as it should be revisited when bitcoin#24356 is backported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK 76a458e
Additional Information
Dependent on backport: merge bitcoin#22278, #24103, #24235, #24177, #24299, #24917, #22564, #25349, #25571, #17487, #26999, #27011 (blockstorage backports: part 2) #6098
Dependent on fix: release unused memory in
CNetMsgMaker::Make()
#6233p2p_ibd_txrelay.py
was first introduced in bitcoin#19423 to test feefilter logic but on account of Dash not having feefilter capabilities, that backport was skipped over but on account of the tests introduced in bitcoin#21327 that test capabilities present in Dash, a minimal version ofp2p_ibd_txrelay.py
has been committed in.vSendMsg
is originally astd::deque
and as an optimization, was changed to astd::list
in 027a852 (dash#3398) but this renders us unable to backport bitcoin#26844 as it introduces build failures. The optimization has been reverted to make way for the backport.Compile failure:
The collection of
CNode
pointers inCConnman::SocketHandlerConnected
has been changed to astd::set
to allow for us to erase elements fromvReceivableNodes
if the node is also in the set of sendable nodes and the send hasn't entirely succeeded to avoid a deadlock (i.e. backport bitcoin#27981)When backporting bitcoin#28165,
denialofservice_tests
has been modified to still check withvSendMsg
instead ofTransport::GetBytesToSend()
as changes in networking code to support LT-SEMs (level-triggered socket events mode) mean that the message doesn't get shifted fromvSendMsg
tom_message_to_send
, as the test expects.Transport::SetMessageToSend()
throughCConnman::SocketSendData()
), not being called during the test runtime.As checking
vSendMsg
(directly or throughnSendMsgSize
) isn't enough to determine if the queue is empty, we now also check withto_send
fromTransport::GetBytesToSend()
to help us make that determination. This mirrors the change present in the upstream backport (source).Breaking Changes
bandwidth.message.*.bytesSent
will no longer include overhead and will now only report message size as specifics that let us calculate the overhead have been abstracted away.Checklist: